feat: add address arg for duckdb-server-rust #649
Conversation
| let addr = SocketAddr::new( | ||
| args.address | ||
| .parse::<IpAddr>() | ||
| .unwrap_or(Ipv4Addr::LOCALHOST.into()), |
There was a problem hiding this comment.
Will this actually fall back since you define a default in line 26? I realize this might be a question for all the defaults here. It would be nice to define them only in one place.
There was a problem hiding this comment.
The above default value (127.0.0.1) will be used if nothing was passed.
The value in unwrap_or (Ipv4Addr::LOCALHOST.into()) will be used if parse fails. For example, if an invalid IP address was passed like --address 127.0
There was a problem hiding this comment.
It would be nice to define them only in one place.
I agree. Do you think using global constant variables a good way to achieve this?
There was a problem hiding this comment.
I didn't know about the parse failure case you described. I wonder whether we just want to fail when the parsing of provided or default values fails. That might make things simpler.
There was a problem hiding this comment.
I just made a change for that. Now, if --address is not provided, it will use default value (only defined in one place), if invalid IP address is provided, it will output the error, not using the default value.
> ./target/release/duckdb-server --address 0.0.0.0 --port 5002
2025-01-09T19:30:34.343971Z INFO duckdb_server: Creating database in ':memory:'
2025-01-09T19:30:34.351430Z WARN duckdb_server: No keys for HTTPS found.
2025-01-09T19:30:34.351448Z INFO duckdb_server: DuckDB Server listening on http://0.0.0.0:5002 and ws://0.0.0.0:5002.
^C
> ./target/release/duckdb-server --port 5002
2025-01-09T19:30:38.984626Z INFO duckdb_server: Creating database in ':memory:'
2025-01-09T19:30:38.991219Z WARN duckdb_server: No keys for HTTPS found.
2025-01-09T19:30:38.991237Z INFO duckdb_server: DuckDB Server listening on http://127.0.0.1:5002 and ws://127.0.0.1:5002.
^C
> ./target/release/duckdb-server --address 999.999.999.999 --port 5002
error: invalid value '999.999.999.999' for '--address <ADDRESS>': invalid IP address syntax
For more information, try '--help'.
There was a problem hiding this comment.
Nice. Should we do the same for the app.rs file (e.g.
)?There was a problem hiding this comment.
Updated. Added the default values as global constants.
Add
--addressas part of CLI arguments. The default is127.0.0.1and if parsing the given value fails, it falls back toIpv4Addr::LOCALHOST